chore: fusdc uses bech32 address hook encoding #10682

0xpatrickdev · 2024-12-11T22:14:17Z

refs: #10614

Description

@agoric/fast-usdc uses bech32 encoded address hooks via decodeAddressHook and encodeAddressHook

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

Updates existing tests. Adds an additional test that shows Advancer forbids extra query parameters

Upgrade Considerations

None, unreleased code

packages/fast-usdc/src/exos/settler.js

packages/cosmic-proto/src/address-hooks.js

cloudflare-workers-and-pages · 2024-12-11T22:23:45Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`6091992`
Status:	✅ Deploy successful!
Preview URL:	https://530bd9a4.agoric-sdk.pages.dev
Branch Preview URL:	https://pc-fusdc-bech32-hooks.agoric-sdk.pages.dev

View logs

turadg · 2024-12-11T22:25:58Z

packages/cosmic-proto/src/address-hooks.js

+const hardenOrFreeze =
+  harden && typeof harden === 'function' ? harden : Object.freeze;


since harden may not be defined,

Suggested change

const hardenOrFreeze =

harden && typeof harden === 'function' ? harden : Object.freeze;

const hardenOrFreeze = typeof harden === 'function' ? harden : Object.freeze;

Thanks these changes are now in #10683

0xpatrickdev · 2024-12-11T23:15:51Z

packages/fast-usdc/src/exos/advancer.js

-              EudParamShape,
-            );
+            const decoded = decodeAddressHook(recipientAddress);
+            mustMatch(decoded, AddressHookShape);


I don't think we check that cctpTxEvidence contains the correct settlementAccountAddress anywhere. At one point I thought this was a concern of the EventFeed, but maybe this is the better place to do it.

To avoid increasing scope of this PR, I'll tackle this in a follow up

dckc

yup, this should work.

The comments are mostly one style suggestion that shows up in several places.

dckc · 2024-12-11T23:41:09Z

packages/fast-usdc/src/cli/transfer.js

+    const encodedAddr = encodeAddressHook(agoricAddr, {
+      EUD: destination,
+    });


minor style point: I'd much rather see:

Suggested change

const encodedAddr = encodeAddressHook(agoricAddr, {

EUD: destination,

});

const encodedAddr = encodeAddressHook(agoricAddr, { EUD });

with the arg renamed to EUD. Do we need 2 names for it?

dckc · 2024-12-11T23:48:59Z

packages/fast-usdc/src/exos/settler.js

+              console.log('no EUD parameter', tx.receiver);
+              return;
+            }
+            if (!matches(EUD, M.string())) {


no need to go outside JS here.

Suggested change

if (!matches(EUD, M.string())) {

if (typeof EUD !== 'string') {

A pattern match for the whole return value of decodeAddressHook(...) would make sense.
Does @endo/patterns have a thing between matches() and mustMatch()? i.e. if this doesn't match, give me a good reason why not, not just a boolean false.

dckc · 2024-12-11T23:50:00Z

packages/fast-usdc/src/exos/settler.js

+          try {
+            const { EUD } = decodeAddressHook(tx.receiver).query;
+            if (!EUD) {
+              console.log('no EUD parameter', tx.receiver);


we want the tracer here, right?

Suggested change

console.log('no EUD parameter', tx.receiver);

log('no EUD parameter', tx.receiver);

This was copied over but yes, updated

dckc · 2024-12-11T23:50:11Z

packages/fast-usdc/src/exos/settler.js

+              return;
+            }
+            if (!matches(EUD, M.string())) {
+              console.log('EUD is not a string', EUD);


Suggested change

console.log('EUD is not a string', EUD);

log('EUD is not a string', EUD);

dckc · 2024-12-11T23:51:03Z

packages/fast-usdc/src/exos/settler.js

@@ -151,17 +151,23 @@ export const prepareSettler = (
            return;
          }

-          if (!addressTools.hasQueryParams(tx.receiver)) {
+          let endUserDestination;


Why a separate name for it?

dckc · 2024-12-11T23:59:52Z

packages/fast-usdc/test/exos/advancer.test.ts

+        {
+          EUD: 'osmo1234',
+          extra: 'value',
+        },


maybe I shouldn't bother with these style nits, but... any particular reason to split this across 4 lines?

Suggested change

{

EUD: 'osmo1234',

extra: 'value',

},

{ EUD: 'osmo1234', extra: 'value' },

dckc · 2024-12-12T00:01:56Z

packages/fast-usdc/test/fast-usdc.contract.test.ts

+      const recipientAddress = encodeAddressHook(settleAddr, {
+        EUD,
+      });


Suggested change

const recipientAddress = encodeAddressHook(settleAddr, {

EUD,

});

const recipientAddress = encodeAddressHook(settleAddr, { EUD });

dckc · 2024-12-12T00:03:22Z

packages/fast-usdc/test/fixtures.ts

+          {
+            EUD: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men',
+          },


feel free to ignore these, but they stick out at me:

Suggested change

{

EUD: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men',

},

{ EUD: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men' },

dckc · 2024-12-12T00:03:39Z

packages/fast-usdc/test/fixtures.ts

+          {
+            EUD: 'dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men',
+          },


Suggested change

{

EUD: 'dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men',

},

{ EUD: 'dydx183dejcnmkka5dzcu9xw6mywq0p2m5peks28men' },

dckc · 2024-12-12T00:03:58Z

packages/fast-usdc/test/fixtures.ts

+          {
+            EUD: 'random1addr',
+          },


Suggested change

{

EUD: 'random1addr',

},

{ EUD: 'random1addr' },

0xpatrickdev commented Dec 11, 2024

View reviewed changes

packages/fast-usdc/src/exos/settler.js Outdated Show resolved Hide resolved

0xpatrickdev commented Dec 11, 2024

View reviewed changes

packages/cosmic-proto/src/address-hooks.js Outdated Show resolved Hide resolved

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch from 3bde1ab to 0637249 Compare December 11, 2024 22:23

turadg reviewed Dec 11, 2024

View reviewed changes

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch 3 times, most recently from a3ba2d1 to f081d9f Compare December 11, 2024 23:10

0xpatrickdev changed the base branch from master to mfig-no-future-address-hooks December 11, 2024 23:10

0xpatrickdev commented Dec 11, 2024

View reviewed changes

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch from f081d9f to b9c275c Compare December 11, 2024 23:18

0xpatrickdev marked this pull request as ready for review December 11, 2024 23:33

0xpatrickdev requested a review from a team as a code owner December 11, 2024 23:33

0xpatrickdev requested review from AgoricTriage, dckc and turadg December 11, 2024 23:33

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch from b9c275c to aa488d8 Compare December 11, 2024 23:38

Agoric deleted a comment from mergify bot Dec 12, 2024

dckc approved these changes Dec 12, 2024

View reviewed changes

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch 2 times, most recently from ba4fce5 to d369f08 Compare December 12, 2024 01:47

michaelfig force-pushed the mfig-no-future-address-hooks branch 2 times, most recently from a8a6172 to 80fee60 Compare December 12, 2024 23:47

Base automatically changed from mfig-no-future-address-hooks to master December 13, 2024 00:39

0xpatrickdev added 2 commits December 12, 2024 19:42

chore: decodeAddressHook throws annotation

e86a5e1

chore: use address-hook encoding

6091992

0xpatrickdev force-pushed the pc/fusdc-bech32-hooks branch from d369f08 to 6091992 Compare December 13, 2024 00:49

0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Dec 13, 2024

mergify bot merged commit 254e849 into master Dec 13, 2024
91 checks passed

mergify bot deleted the pc/fusdc-bech32-hooks branch December 13, 2024 01:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: fusdc uses bech32 address hook encoding #10682

chore: fusdc uses bech32 address hook encoding #10682

0xpatrickdev commented Dec 11, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Dec 11, 2024 •

edited

Loading

turadg Dec 11, 2024

0xpatrickdev Dec 11, 2024

0xpatrickdev Dec 11, 2024 •

edited

Loading

dckc left a comment

dckc Dec 11, 2024

dckc Dec 11, 2024

dckc Dec 11, 2024

0xpatrickdev Dec 12, 2024

dckc Dec 11, 2024

dckc Dec 11, 2024

0xpatrickdev Dec 12, 2024

dckc Dec 11, 2024

dckc Dec 12, 2024

dckc Dec 12, 2024

dckc Dec 12, 2024

dckc Dec 12, 2024

		const hardenOrFreeze =
		harden && typeof harden === 'function' ? harden : Object.freeze;

	const hardenOrFreeze =
	harden && typeof harden === 'function' ? harden : Object.freeze;
	const hardenOrFreeze = typeof harden === 'function' ? harden : Object.freeze;

	if (!matches(EUD, M.string())) {
	if (typeof EUD !== 'string') {

	console.log('no EUD parameter', tx.receiver);
	log('no EUD parameter', tx.receiver);

	console.log('EUD is not a string', EUD);
	log('EUD is not a string', EUD);

chore: fusdc uses bech32 address hook encoding #10682

chore: fusdc uses bech32 address hook encoding #10682

Conversation

0xpatrickdev commented Dec 11, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Dec 11, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev commented Dec 11, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Dec 11, 2024 •

edited

Loading

0xpatrickdev Dec 11, 2024 •

edited

Loading